Skip to content
This repository has been archived by the owner on Dec 14, 2021. It is now read-only.

Update application services to 0.48.0 - Xcode 11.3 #1178

Merged
merged 11 commits into from
Feb 6, 2020

Conversation

kaylagalway
Copy link
Contributor

@kaylagalway kaylagalway commented Jan 7, 2020

Fixes #1176
Fixes #1170
Fixes #955

@kaylagalway kaylagalway added this to the 1.7.2 milestone Jan 7, 2020
@kaylagalway kaylagalway requested a review from a team as a code owner January 7, 2020 21:36
@kaylagalway kaylagalway self-assigned this Jan 7, 2020
@kaylagalway
Copy link
Contributor Author

Builds on buddybuild are currently failing because Glean needs to be updated to build with the newest version of Xcode and Swift. There should be a new release with this update in the next day or two. Glean team ticket here: https://bugzilla.mozilla.org/show_bug.cgi?id=1607593

Copy link

@eliserichards eliserichards left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! 🚀

Copy link
Contributor

@jhugman jhugman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. A couple of nits, which are mainly down to taste. Well done.

if let url = FileManager.default.containerURL(forSecurityApplicationGroupIdentifier: sharedContainerIdentifier) {
rootPath = url.path
guard let loginsDatabasePath = loginsDatabasePath else { return }
setupSalt()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: my preference would be to have setupSalt() return a string, and assignment happen here.

e.g.

self.salt = findSalt()

} else {
print("Unable to find the shared container. Defaulting profile location to ~/Documents instead.")
rootPath = (NSSearchPathForDirectoriesInDomains(.documentDirectory, .userDomainMask, true)[0])
salt = setupPlaintextHeaderAndGetSalt(databasePath: loginsDatabasePath, encryptionKey: loginsKey)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: similarly, this naming could do with some work:

salt = createNewSalt(databasePath: loginsDatabasePath, encryptionKey: loginsKey) 

Copy link
Contributor

@jhugman jhugman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The AppDelegate.isFirstRun calculated property may well cause some problem on upgrade.

Test the upgrade path, if it works! Then! Big! Green! Button!

@kaylagalway kaylagalway merged commit 21e674d into master Feb 6, 2020
@jhugman jhugman deleted the update-app-services branch February 7, 2020 14:51
@kaylagalway kaylagalway changed the title Update application services to 0.48.0 Update application services to 0.48.0 - Xcode 11.3 Feb 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants